fix(bns): correct SQLite read-width predicate and make_decrypted helper - #196

Summary

Two correctness bugs in src/cryptonote_core/beldex_name_system.cpp.

1. SQLite read-width predicate compares bytes against bits

At lines 301-306, the templated get<T> overloads dispatch between sqlite3_column_int (32-bit) and sqlite3_column_int64 (64-bit) based on sizeof(T). The current predicates compare sizeof(T) to 32:

// Small (<=32 bits) integers
template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) <= 32), int> = 0>
T get(sql_compiled_statement& s, int index) { return static_cast<T>(sqlite3_column_int(s.statement, index)); }

// Big (>32 bits) integers
template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) > 32), int> = 0>
T get(sql_compiled_statement& s, int index) { return static_cast<T>(sqlite3_column_int64(s.statement, index)); }

sizeof returns bytes, not bits. Every integral type sits comfortably under 32 bytes, so the first overload matches all of them — every column, including uint64_t heights and timestamps, is read through sqlite3_column_int (signed 32-bit). Values above INT32_MAX are silently truncated.

The comment on the first template says "Small (<=32 bits)", confirming the intent was 32 bits = 4 bytes. The sibling bind<T> overloads at line 178 already use the correct predicate:

// Small (<=4 bytes), bind via sqlite3_bind_int
template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) <= 4), int> = 0>

So the get/bind pair is already inconsistent. Mainnet heights and timestamps dont yet exceed 2^31, so this is latent: the truncation will start happening silently when the chain or the time epoch grows past INT32_MAX.

2. make_decrypted calls encrypt instead of decrypt

At lines 1584-1590:

mapping_value mapping_value::make_decrypted(std::string_view name, const crypto::hash* name_hash) const
{
  mapping_value result{*this};
  result.encrypt(name, name_hash);                  // should be decrypt
  assert(!result.encrypted);                        // will never hold after encrypt()
  return result;
}

The function name says decrypt; the function body calls encrypt. The assert at line 1588 (!result.encrypted) is inconsistent with calling encrypt and would fire in any build with asserts enabled if this function were used.

mapping_value::decrypt has signature bool decrypt(std::string_view name, mapping_type type, const crypto::hash* name_hash = nullptr) (see beldex_name_system.h:95), so the helper needs to accept a mapping_type argument to forward. The sibling make_encrypted is correctly structured.

The function has no callers right now (grep shows only the declaration in the header and the definition here), so this is a quiet correctness bug in dormant API surface. Worth fixing before anything starts depending on it.

Patches

sizeof predicate fix

 // Small (<=32 bits) integers
-template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) <= 32), int> = 0>
+template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) <= 4), int> = 0>
 T get(sql_compiled_statement& s, int index) { return static_cast<T>(sqlite3_column_int(s.statement, index)); }

 // Big (>32 bits) integers
-template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) > 32), int> = 0>
+template <typename T, std::enable_if_t<std::is_integral_v<T> && (sizeof(T) > 4), int> = 0>
 T get(sql_compiled_statement& s, int index) { return static_cast<T>(sqlite3_column_int64(s.statement, index)); }

This mirrors the existing bind<T> overloads. No behavior change for current heights; correct dispatch once values cross 2^31.

make_decrypted fix

src/cryptonote_core/beldex_name_system.cpp:

-mapping_value mapping_value::make_decrypted(std::string_view name, const crypto::hash* name_hash) const
+mapping_value mapping_value::make_decrypted(std::string_view name, mapping_type type, const crypto::hash* name_hash) const
 {
   mapping_value result{*this};
-  result.encrypt(name, name_hash);
+  result.decrypt(name, type, name_hash);
   assert(!result.encrypted);
   return result;
 }

src/cryptonote_core/beldex_name_system.h, update declaration to match:

-  mapping_value make_decrypted(std::string_view name, const crypto::hash* name_hash = nullptr) const;
+  mapping_value make_decrypted(std::string_view name, mapping_type type, const crypto::hash* name_hash = nullptr) const;

mapping_type is non-defaultable here because decrypt requires it. Since the function has no current callers this is a safe API correction.

Why these two together

Both are in the same file, both are one-token-style correctness fixes, both touch BNS data handling rather than memory safety. Splitting them just doubles the review surface for two one-token fixes.

Tests

The first change is dispatch-only and produces the same column reads at current heights; the second has no callers to break. Happy to add a test that binds and reads back a uint64_t > INT32_MAX to lock in the dispatch fix.